Open
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds version 3.0.16 of nodeeditor to the xmake.lua package definition. However, this version introduces significant breaking changes—including a requirement for C++17, modified header paths, and API removals—that are incompatible with the current build and test configurations in the script.
|
|
||
| set_urls("https://github.com/paceholder/nodeeditor/archive/refs/tags/$(version).tar.gz", | ||
| "https://github.com/paceholder/nodeeditor.git") | ||
| add_versions("3.0.16", "453e6eb783379fee6edf9282283576eaa7d27349b8731e638926ccbd8331f7ef") |
Contributor
There was a problem hiding this comment.
Adding version 3.0.16 is problematic because it is a major rewrite (3.x) with breaking changes that are not handled by the current xmake.lua script. Specifically:
- C++ Standard: Version 3.x requires C++17, but the
on_testblock (line 48) is configured forc++14. - Header Paths: Includes have moved from
nodes/toQtNodes/. Theon_testblock will fail to find the headers. - API Changes:
QtNodes::DataModelRegistry(line 45) has been removed in 3.x. - Qt Version: The script is hardcoded to Qt5 (lines 17, 26, 32), while 3.x supports and often targets Qt6.
- Patches: The existing patches for 2.x (lines 11-12) likely do not apply to 3.x, and new ones might be needed.
The build logic and test snippets need to be updated to support both 2.x and 3.x before this version can be safely added.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New version of nodeeditor detected (package version: 2.2.2, last github version: 3.0.16)